slicer69/doas: multiple security issues

There are a number of secruity issues I found with the doas port of slicer69.

Buffer overflow (privilege escalation to root)

doas(1) concatenates all supplied arguments using strlcpy(3) and strlcat(3), the maintainer replace them with strncpy(3) and strncat(3) respectively without knowing the different semantics.

The maintainer applied the suggested changes with a misleading commit message.

Updated license file.

Fixes potential buffer overflow on Linux systems. (Thanks to Duncaen for pointing out the issue.)

Not only was the first line used to hide the actual change, but it also downplays the impact if someone actually reads the whole commit message. The buffer overflow was exploitable and leads to local root privileges, even for users that were not allowed to use doas(1) in the configuration file.

I did not request a CVE for this bug at the time.

Broken UID parsing falls back to root (CVE-2019-15900)

Incorrect group change behaviour (CVE-2019-15901)

In the same mail thread about the buffer overflow I also reported that replacing setusercontext(3) with the USER_SETLOGIN and USER_SETGROUP flags requires a replacement that does setuid(2), setgid(2) and initgroups(3) and not just setuid(2). At the time the maintainer refused to fix it and tried to explain that it works as intended.

Later in 2019 the doas port was added to the pkgsrc collection, I tried to notify the pkgsrc maintainer and the port maintainer in the github PR, but my comment was promptly deleted without a response.

After a week I decided to not wait longer and just provide a patch for this issue. I opened PR#23 and explained again why this change is necessary and why it is a vulnerability.

The maintainer merged the PR but refused to acknowledge the security impact:

I have a point of contention with the overall view. One being that I’m not sure this qualifies as a security concern. Having the group of the calling user is arguably expected, or at least certainly useful. However, I can see why, in some cases it would also be dangerous. I’m going to commit this patch and test it. Thank you for putting it together.